Skip to content

fix(prompts): calculate multi-select vertical offsets using post-wrapped line counts#577

Open
HarshalPatel1972 wants to merge 2 commits into
bombshell-dev:mainfrom
HarshalPatel1972:fix/multiselect-overflow
Open

fix(prompts): calculate multi-select vertical offsets using post-wrapped line counts#577
HarshalPatel1972 wants to merge 2 commits into
bombshell-dev:mainfrom
HarshalPatel1972:fix/multiselect-overflow

Conversation

@HarshalPatel1972

Copy link
Copy Markdown

What does this PR do?

This PR corrects a terminal rendering bug in the MultiSelectPrompt where multi-line choices containing wrapped description strings caused terminal cursor line-tracking offsets to miscalculate. By forcing vertical layout boundaries to evaluate height metrics directly from post-wrapped line array lengths rather than raw element counts, the terminal layout accurately positions the cursor and prevents option text from visually overwriting itself on smaller displays.

Closes #

Type of change

  • Bug fix
  • Feature
  • Refactor (no behavior change)
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Checklist

  • I have read the contributing guidelines
  • My code follows the code style of this project
  • I have performed a self-review of my own code
  • My changes generate no new lint or build warnings

Copilot AI review requested due to automatic review settings June 25, 2026 23:10
@changeset-bot

changeset-bot Bot commented Jun 25, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 9b3f3be

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@clack/prompts Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@gameroman gameroman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a related issue?

If this is a bug fix, we need tests

Please fill in the pull request template fully and do not remove any sections

@pkg-pr-new

pkg-pr-new Bot commented Jun 26, 2026

Copy link
Copy Markdown

commit: 62ce5a4

@dreyfus92 dreyfus92 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @HarshalPatel1972, thank you for taking the time in working with this but please follow the PR template provided rather than whatever your agent outputs:
https://github.com/bombshell-dev/clack/blob/main/.github/PULL_REQUEST_TEMPLATE.md

…rminals

This corrects a terminal rendering bug in the MultiSelectPrompt (Issue bombshell-dev#116).
Previously, terminal row padding calculations contained two offsetting errors:
1. \	itleLineCount\ was overestimating by 1 due to trailing newlines.
2. \ooterLineCount\ was underestimating on narrow terminals because the footer string wasn't properly wrapped.

When the terminal filled up, this offset math caused the \limitOptions\ slicing window to miscalculate available space by 1 row, causing the prompt to overflow the terminal and visually overwrite itself on redraw.

This fix correctly evaluates both bounds using \wrapTextWithPrefix\ and ensures small terminals gracefully constrain multiselect options.

Signed-off-by: Harshal Patel <hp842484@gmail.com>
@HarshalPatel1972 HarshalPatel1972 force-pushed the fix/multiselect-overflow branch from 62ce5a4 to 1a1828f Compare July 4, 2026 11:49
Signed-off-by: Harshal Patel <hp842484@gmail.com>
@HarshalPatel1972

Copy link
Copy Markdown
Author

@gameroman @dreyfus92 Apologies for the messy first attempt, you were absolutely right. I've re-written the implementation from scratch, properly updated the PR body to exactly match the template, and pushed terminal constraint tests that use MockWritable to simulate narrow environments.

As for the related issue, this closes #116, where the multiselect component overwrites itself when scrolling through long lists. The root cause was that footerLineCount was being calculated without wrapping the footer string, under-reporting the necessary padding to limitOptions. Let me know if you need any other changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants